-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Namespaces included by labelSelector act as IncludedNamespaces for Backup #8342
base: main
Are you sure you want to change the base?
Conversation
25e190c
to
d311933
Compare
5e2355e
to
f2f044b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8342 +/- ##
==========================================
- Coverage 59.39% 59.39% -0.01%
==========================================
Files 370 370
Lines 39987 40010 +23
==========================================
+ Hits 23750 23763 +13
- Misses 14745 14753 +8
- Partials 1492 1494 +2 ☔ View full report in Codecov by Sentry. |
I understand there is a requirement from the community to back up resources under the labelled namespace, but we can also do that by using the namespace filter, do we need a new way to do the same function? |
Some admins may find it easier to automate ns backup by label. Apart from velero issue this pr fixes, it has also been requested to our team in https://issues.redhat.com/browse/OADP-4572 I think the question is, when is the current behavior of backing up namespace but not namespaced resources within it desirable? The previous fix already decided that any namespace not included by namespace filters like label should not be in the backup. Now we're just saying every namespace included via namespace filters (including label) will have namespaced resources of those namespaces in the backup. |
@kaovilai Thanks for this PR, but have we reached agreement in terms of whether we should implement it? |
I don't think we have reached agreements yet. |
@kaovilai I agree that only backing up the namespace with selected labels without any resources in it is not a common use case, but at least, we have that option. After introducing this change, we lost that option. If the introduced method is the preferable option, we can go forward that way, but before applying it, I think we need to make sure that. |
Another thing with this implementation is that namespaces included via label selector would not show up in |
@kaovilai v1.16 candidate? |
Please add this PR to v1.16 milestone |
8360892
to
f2490fe
Compare
every item under NS is included. Signed-off-by: Tiger Kaovilai <[email protected]>
Thank you for contributing to Velero!
Please add a summary of your change
Namespaces included by backup's selector behave the same as specifying backup.IncludedNamespaces. All resources under the labeled namespace is included.
Prior to this change, labeled namespaces are included, but anything else under the namespace if not labeled are not.
Does your change fix a particular issue?
Fixes #7492
Please indicate you've done the following:
make new-changelog
) or comment/kind changelog-not-required
on this PR.site/content/docs/main
.